[misc][torchair] fix bugs around deepseek mtp, enable_shared_expert_dp and use_cached_kv_cache_bytes
#3074
Conversation
|
👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:
If CI fails, you can run linting and testing checks locally according Contributing and Testing. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a bug in DeepSeek's Multi-Token Prediction (MTP) when enable_shared_expert_dp is active. The core of the fix involves conditionally using TorchairDeepSeekMTP for this configuration. Additionally, the logic for handling attention metadata has been updated to be more robust by avoiding a hardcoded dictionary key. My review points out a potential crash in this new metadata handling and provides a suggestion for a safer implementation.
Signed-off-by: linfeng-yuan <1102311262@qq.com>
linfeng-yuan
force-pushed
the
fix_deepseek_mtp_with_shared_expert_dp
branch
from
9669e03 to
b359070
Compare
…ange in NPUModelRunner Signed-off-by: linfeng-yuan <1102311262@qq.com>
…_cache_bytes disabled Signed-off-by: linfeng-yuan <1102311262@qq.com>
linfeng-yuan
changed the title
deepseek mtp, enable_shared_expert_dp and use_cached_kv_cache_bytes
linfeng-yuan
added
ready
|
Hello! You've invoked me with |
linfeng-yuan
removed
ready
linfeng-yuan
added
ready
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces several targeted fixes for bugs related to deepseek_mtp, enable_shared_expert_dp, and KV cache memory management. The changes correctly adjust MoE state handling and metadata synchronization for shared_expert_dp, prevent an unnecessary reduction in available KV cache memory when use_cached_kv_cache_bytes is disabled, and improve the robustness of accessing attention metadata. The fixes appear correct and well-aligned with the stated objectives. I have no major concerns with this pull request.
Signed-off-by: linfeng-yuan <1102311262@qq.com>
linfeng-yuan
force-pushed
the
fix_deepseek_mtp_with_shared_expert_dp
branch
from
1d27f71 to
6fd21e8
Compare
|
@wangxiyuan I pushed a new commit to tackle the last problem with large ep deployments and fix the UT break. Please review this commit and check whether there is any blocking problem to merge this pr. |
What this PR does / why we need it?
This miscellaneous contains several small fixes:
shared_expert_dpenabled.use_cached_kv_cache_bytesdisabled.fused_moe_statefromMC2toAll2Allsince the padding logic ofmc2_maskis incompatible with input hidden_states whenshared_expert_dpenabled.Once this PR is merged, users can launch disaggregated_prefill deployments (large_ep) with
deepseek_mtpandshared_expert_dpasv0.9.1-devbranch. The remaining problem of kv_cache tokens decline compared tov0.9.1-devwill be resolved by #3073.Does this PR introduce any user-facing change?
No.
How was this patch tested?
E2E vllm serving about deepseek_mtp with torchair graph mode and
enable_shared_expert_dpwith eager mode. Large ep deployments are also tested with this PR.